[UI Test] - Add validation on new product screen and new add product tests #8407

jostnes · 2022-12-14T08:29:05Z

Description

This PR adds a new validation (verifyProductTypeScreenLoaded()) for the add product test to validate that the correct product screen is displayed. This also adds 2 new add product (virtual and variable) UI tests.

Testing instructions

Tests should pass in CI and locally. The tests should also fail when the expected fields are not displayed on screen.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

wpmobilebot · 2022-12-14T08:44:30Z

You can test the changes from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr8407-c9a64bc on your iPhone

If you need access to App Center, please ask a maintainer to add you.

pachlava

Wow, that's a lot of new tests, @jostnes 👍

I left some comments, but they are nitpicks, nothing really serious. I'm approving the PR as is, and you can adjust it as you like.

Thank you 🙇

pachlava · 2022-12-14T15:43:28Z

WooCommerce/UITestsFoundation/Screens/Products/ProductsScreen.swift

+        let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)
+        app.staticTexts.containing(productTypeLabel).firstMatch.tap()


Just thinking aloud: the buttons for product type selection have no identifiers (as of now), so identifiers can't be used (which would be justified, since this non-asserting function can potentially be used in tests for localizations). So, we can go for exact text values (as you did before). Is there a reason for less strict locator (only part of a string / case-insensitive) that you're using now?

yeah i actually tried to add identifiers for the cells but it looks like it will take more effort than i anticipated so i dropped that for now.

so the reason is that i wanted to be able to reuse it in the test like this:

... .selectProductType(productType: productType) .verifyProductTypeScreenLoaded(productType: productType) ...

the test wants the product type in both, using simple physical product as an example, in select product the text for that is Simple physical product and in add product screen it's Physical, the common denominator in both is the word physical (case-insensitive) so i decided to go with that. it fits the needs of the test case while still keeping it reusable and clear.

pachlava · 2022-12-14T15:45:42Z

WooCommerce/UITestsFoundation/Screens/Products/SingleProductScreen.swift

+        let priceLabel = NSPredicate(format: "label CONTAINS[c] %@", "price")
+        let inventoryLabel = NSPredicate(format: "label CONTAINS[c] %@", "inventory")
+        let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)
+        let addVariationLabel = NSPredicate(format: "label CONTAINS[c] %@", "add variations")


Since this is an asserting function, I think it would benefit from more strict matching (this is a tested code):

Suggested change

let priceLabel = NSPredicate(format: "label CONTAINS[c] %@", "price")

let inventoryLabel = NSPredicate(format: "label CONTAINS[c] %@", "inventory")

let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)

let addVariationLabel = NSPredicate(format: "label CONTAINS[c] %@", "add variations")

let priceLabel = NSPredicate(format: "label == 'Add Price'")

let inventoryLabel = NSPredicate(format: "label == 'Inventory'")

let productTypeLabel = NSPredicate(format: "label ==[c] '\(productType)'")

let addVariationLabel = NSPredicate(format: "label == 'Add variations'")

Also, it would be easier to understand what exactly is missing, if an assertion fails.

updated in c9a64bc

pachlava · 2022-12-14T15:47:22Z

WooCommerce/UITestsFoundation/Screens/Products/SingleProductScreen.swift

+
+        // different product types displays different fields on add product screen
+        // this is to validate that the correct screens are displayed
+        switch productType {
+        case "physical", "virtual":
+            XCTAssertTrue(app.staticTexts.containing(priceLabel).firstMatch.exists)
+            XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
+        case "variable":
+            XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)
+            XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)


inventoryLabel is present for both cases, so it can be moved to common?

Suggested change

// different product types displays different fields on add product screen

// this is to validate that the correct screens are displayed

switch productType {

case "physical", "virtual":

XCTAssertTrue(app.staticTexts.containing(priceLabel).firstMatch.exists)

XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)

case "variable":

XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)

XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)

XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)

// different product types displays different fields on add product screen

// this is to validate that the correct screens are displayed

switch productType {

case "physical", "virtual":

XCTAssertTrue(app.staticTexts.containing(priceLabel).firstMatch.exists)

case "variable":

XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)

ah ok, this was written with the future in mind, the next product type (grouped) we introduce, inventoryLabel will no longer be a common field so i'll leave it as is for now.

Thanks! This is what I thought about, just wanted to be sure 🙂

pachlava · 2022-12-14T15:48:44Z

WooCommerce/UITestsFoundation/Screens/Products/SingleProductScreen.swift

    }
+
+    public func verifyProductTypeScreenLoaded(productType: String) throws -> Self {
+        let priceLabel = NSPredicate(format: "label CONTAINS[c] %@", "price")


WDYT of renaming priceLabel to addPriceLabel, as done for addVariationLabel?

updated in c9a64bc

pachlava · 2022-12-14T15:49:46Z

WooCommerce/UITestsFoundation/Screens/Products/SingleProductScreen.swift

+            XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)
+            XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
+        default:
+            // fail test if product type doesn't exist


I think the line below this comment has a good descriptive error message, so it's not necessary:

Suggested change

// fail test if product type doesn't exist

removed in c9a64bc

pachlava · 2022-12-14T15:52:10Z

WooCommerce/WooCommerceUITests/Tests/ProductsTests.swift

+    func test_add_simple_virtual_product() throws {
+        try ProductFlow.addAndVerifyNewProduct(productType: "virtual")
+    }
+
+    func test_add_variable_product() throws {
+        try ProductFlow.addAndVerifyNewProduct(productType: "variable")
    }


➕ for extracting the repetitive part to a flow. Nice 👍

peril-woocommerce · 2022-12-15T05:12:49Z

	Warnings
⚠️	PR is not assigned to a milestone.

Generated by 🚫 dangerJS

add validation on new product screen and new ui tests

1118ad3

jostnes requested a review from a team as a code owner December 14, 2022 08:29

jostnes added feature: product details Related to adding or editing products, including Product Settings. category: ui tests Related to UI testing. labels Dec 14, 2022

pachlava approved these changes Dec 14, 2022

View reviewed changes

use equal instead of contains, rename for consistency

c9a64bc

jostnes added this to the 11.7 milestone Dec 15, 2022

jostnes merged commit c2bb7fe into trunk Dec 15, 2022

jostnes deleted the add-virtual-product-ui-test branch December 15, 2022 06:23

		let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)
		app.staticTexts.containing(productTypeLabel).firstMatch.tap()

[UI Test] - Add validation on new product screen and new add product tests #8407

[UI Test] - Add validation on new product screen and new add product tests #8407

Uh oh!

Conversation

jostnes commented Dec 14, 2022

Description

Testing instructions

Uh oh!

wpmobilebot commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pachlava left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peril-woocommerce bot commented Dec 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wpmobilebot commented Dec 14, 2022 •

edited

Loading